feat(gmail): add --include-attachments to messages search - #962
feat(gmail): add --include-attachments to messages search#962ronny-rentner wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54c58df304
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| items, err := fetchMessageDetails(ctx, svc, messages, idToName, loc, c.IncludeBody, c.BodyFormat) | ||
| items, err := fetchMessageDetails(ctx, svc, messages, idToName, loc, c.IncludeBody, c.BodyFormat, c.IncludeAttachments) |
There was a problem hiding this comment.
Render attachments in non-JSON search output
When --include-attachments is used with the default table output or --plain, this path fetches and populates messageItem.Attachments, but the later call to gmailMessageColumns(c.IncludeBody, c.Full) never reads that field. The flag therefore performs the additional full-format API requests while producing exactly the same stdout as a search without the flag. Pass the option into the presentation layer and render the metadata, or explicitly restrict and document the flag as JSON-only.
Useful? React with 👍 / 👎.
|
Codex review: found issues before merge. Reviewed August 8, 2026, 12:04 PM ET / 16:04 UTC. ClawSweeper reviewWhat this changesAdds an opt-in Gmail message-search flag and environment variable that return attachment metadata without rendering decoded message bodies. Merge readinessKeep open for a maintainer product-direction decision. The feature has sufficient redacted live CLI proof, but attachment filenames and MIME values are written unescaped into the table, so a tab or line break can corrupt text/ Likely related people: Peter Steinberger (high confidence) and chrischall (medium confidence) from the Gmail search and attachment-output history. Priority: P2 Review scores
Verification
How this fits togetherGmail message search lists matching IDs, fetches message details when needed, and renders JSON or a text table. This option extends the detail-fetch and presentation stages to expose attachment metadata. flowchart LR
A[Gmail search query] --> B[Matching message IDs]
B --> C[Message detail fetch]
C --> D[Attachment metadata collection]
D --> E[JSON result]
D --> F[Text or plain table]
Decision needed
Why: Current main and the latest release do not establish this command or environment-variable contract, so acceptance is a product decision rather than a repair of documented behavior. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: If a maintainer sponsors this opt-in CLI surface, preserve the shared JSON attachment shape and sanitize attachment text metadata before it enters the human or plain table renderer. Do we have a high-confidence way to reproduce the issue? Yes, from source: a Gmail attachment filename or MIME value containing a tab, carriage return, or newline reaches the new table cell unchanged, while the table writer uses those characters as output structure. Is this the best way to solve the issue? No: the feature design needs explicit maintainer sponsorship, and the new table cell must escape control characters to preserve parseable text output. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8fe3e7995d0b. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (12 earlier review cycles; latest 8 shown)
|
…t/deep parts in attachment mask
…meType/id in text search output
…n the search text column
…stom attachment field mask
…h text column (filename, mimeType, size only)
|
On the "render MIME type and identifier in text output" finding: mimeType is now in the 1. The full id doesn't fit a table column. Gmail's 2. A short id isn't feasible either. gogcli already uses a short (first-8-char) form of the and an id captured in one call still downloaded fine in a separate later call. Because the id changes on every call, its short form changes on every search too — it would be neither stable nor a match for a later download's filename, so it conveys nothing a caller can act on. So the column shows |
Summary
Adds
--include-attachmentstogmail messages search. With it, each result carries its attachment metadata — filename, size, mimeType (and theattachmentIdin JSON) — so you can see what a message holds without switching to--include-body.Today the only way to see a message's attachments in search results is
--include-body, which also decodes and renders the full body.--include-attachmentsgives just the attachment list.--include-attachments(envGOG_GMAIL_INCLUDE_ATTACHMENTS).attachmentOutputshape (the same one--include-bodyalready produces), so the JSON is purely additive — no existing field changes.ATTACHMENTScolumn (filename (mimeType, size)); JSON gains theattachmentsarray (with theattachmentId).format=full— the same complete fetchgmail get/thread/draftsalready use — so attachments at any MIME nesting depth are listed. The body data is transferred but never rendered.Motivation
Lets a caller enumerate what a message carries — and, from the JSON, get the
attachmentIdto download — without switching to--include-body.User-facing changes
--include-attachmentsongmail messages search.GOG_GMAIL_INCLUDE_ATTACHMENTS(mirrors the existingGOG_GMAIL_INLINE_MAX_BYTESpattern).ATTACHMENTScolumn in text output and an additiveattachmentsarray in JSON.Testing
TestExecute_GmailMessagesSearch_IncludeAttachments: lists an attachment nested several MIME levels down (proving the full fetch), does not render the body, and asserts the fetch usesformat=fullwith no capping parts mask; the default search lists neither.make ciclean (fmt / lint / deadcode / tests / docs).Verification
Redacted terminal output from the built binary against a live Gmail account (ids masked,
attachmentIdtruncated; sizes, mimeType, sender, subject verbatim).Text output — the
ATTACHMENTScolumn:JSON — same metadata plus the per-fetch
attachmentId: